-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Fix compat between libc++ and emscripten's xlocale.h #23414
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -25,8 +27,6 @@ | |||
# include <__locale_dir/locale_base_api/fuchsia.h> | |||
#elif defined(__wasi__) || defined(_LIBCPP_HAS_MUSL_LIBC) | |||
# include <__locale_dir/locale_base_api/musl.h> | |||
#elif defined(__APPLE__) || defined(__FreeBSD__) | |||
# include <xlocale.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would adding emscripten here be less of a diff, so less churn in the next libc++ update? Or must the change be on top?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not possible because we would already have hit the _LIBCPP_HAS_MUSL_LIBC case above. Our case needs to come first.
libc++ was assuming that musl doesn't have `xlocale.h` which is true, but emscripten adds xlocale.h for compatibility (I'm not sure we really need to, but we do). Fixes: emscripten-core#23413
Before, the code was like https://github.com/emscripten-core/emscripten/blob/dc1abd514b1bade135a01a4453a9ff6def0793b6/system/lib/libcxx/include/__locale_dir/locale_base_api.h#L12-L30 But now they are divided into two parts, one using the new API and the other using old. See "Locale API reimplementation" above. https://github.com/llvm/llvm-project/blob/ec28b8f9cc7f2ac187d8a617a6d08d5e56f9120e/libcxx/include/__locale_dir/locale_base_api.h#L116-L138 This adds Emscripten in the beginning of the "old" API list. (This has to be the beginning; see emscripten-core#23414)
Before, the code was like https://github.com/emscripten-core/emscripten/blob/dc1abd514b1bade135a01a4453a9ff6def0793b6/system/lib/libcxx/include/__locale_dir/locale_base_api.h#L12-L30 But now they are divided into two parts, one using the new API and the other using old. See "Locale API reimplementation" above. https://github.com/llvm/llvm-project/blob/ec28b8f9cc7f2ac187d8a617a6d08d5e56f9120e/libcxx/include/__locale_dir/locale_base_api.h#L116-L138 This adds Emscripten in the beginning of the "old" API list. (This has to be the beginning; see emscripten-core#23414)
This updates libcxx and libcxxabi to LLVM 20.1.4: https://github.com/llvm/llvm-project/releases/tag/llvmorg-20.1.4 Before going into each additional change of the PR, these are some noteworthy changes from this LLVM release: - Freezing C++03 headers - RFC: https://discourse.llvm.org/t/rfc-freezing-c-03-headers-in-libc/77319 - PRs: - llvm/llvm-project#108999 - llvm/llvm-project#109000 - llvm/llvm-project#109001 - llvm/llvm-project#109002 - This copies libc++ headers of the last LLVM 19 release into a separate directory (https://github.com/llvm/llvm-project/tree/main/libcxx/include/__cxx03) and redirects all C++03 workflow there. The motivation is not to fix C++03 related changes unless they are critical bugs, and simplifies the main headers. So the main headers are like ``` #if __cplusplus < 201103L && defined(_LIBCPP_USE_FROZEN_CXX03_HEADERS) # include <__cxx03/algorithm> #else ... main header content ... ``` As you can see it looks like we can avoid opting into this by not defining `_LIBCPP_USE_FROZEN_CXX03_HEADERS` at the moment. I think their eventual goal is to remove C++03 support from the main headers but it hasn't seem to have happened yet and I don't know what their timeline for that is. Adding that `__cxx03` directory increases the header size by 10MB. We can get away not using them by not defining `_LIBCPP_USE_FROZEN_CXX03_HEADERS` in this release, so this update does not include that directory. - Sharing of headers between libc++ and libc (Project Hand in Hand) - RFC: https://discourse.llvm.org/t/rfc-project-hand-in-hand-llvm-libc-libc-code-sharing/77701 - PR: llvm/llvm-project#91651 (More are likely to come) This tries to share some libc headers from libcxx. libcxx's source files can depend on headers from `libc/shared`, `libc/src/__support`, `libc/include/llvm-libc-macros`, and `libc/include/llvm-libc-types`. And it turns out libcxx can also depend on `libc/hdr`, even though the directory is not in the diagram in the RFC. These headers can be only included from `libcxx/src` and not `libcxx/include`, to prevent libcxx's API from changing. So these headers don't need to be copied into `cache/`. - Locale API reimplementation It doesn't seem to have an RFC, but the PRs are: - llvm/llvm-project#113737 - llvm/llvm-project#115176 - llvm/llvm-project#115752 - llvm/llvm-project#122489 It looks this aims provide a new way to define locale API for each platform. Currently Apple, FreeBSD, MSVCRT, and Fuschia are using the new API and the others are still using the old one: https://github.com/llvm/llvm-project/blob/ec28b8f9cc7f2ac187d8a617a6d08d5e56f9120e/libcxx/include/__locale_dir/locale_base_api.h#L116-L138 For our purpose, adding `if defined(__EMSCRIPTEN__)` entry to the list of "old" list seems to work for the moment, but we may need to move to the new way eventually later. --- Additional changes: - Copy `vendor/llvm/default_assertion_handler.in` to `__assertion_handler`: aa53648 Our previous `__assertion_handler` was copied from `libcxx/vendor/llvm/default_assertion_handler.in`. This updates our `__assertion_handler` to the new `libcxx/vendor/llvm/default_assertion_handler.in`. For what this file is, see the PR description of #22994, with which the file was added. - Remove `libcxx/include/__cxx03` directory: d646f6b As I described in "Freezing C++ headers" above, C++03 headers were copied to `include/cxx03` to be "frozen". But by not defining `_LIBCPP_USE_FROZEN_CXX03_HEADERS` we can still use the main headers. This new `__cxx03` header directory is almost 10M and we are not using it in this update, this deletes it. - Define more variables in `__config_site`: 9912236 libcxx decides to almost all configuration macros to be defined. So before it tested whether a macro was defined/undefined, and now it assumes it is defined and tests whether its value is 1/0. Before llvm/llvm-project#112094 `_config_site.in` used to use `#cmakedefine`, which only defined variables when they were enabled, but now it uses `#cmakedefine01`, which always defines variables and assigns 1 or 0 depending on whether the feature is enabled. So this change adds `#define` to each variable that `_config_site.in` has an entry of. The value assigned is 1 if it was defined in our previous Emscripten environment and 0 if not. - Fix Emscripten's locale: 2ae01b0 Before, the code was like https://github.com/emscripten-core/emscripten/blob/dc1abd514b1bade135a01a4453a9ff6def0793b6/system/lib/libcxx/include/__locale_dir/locale_base_api.h#L12-L30 But now they are divided into two parts, one using the new API and the other using old. See "Locale API reimplementation" above. https://github.com/llvm/llvm-project/blob/ec28b8f9cc7f2ac187d8a617a6d08d5e56f9120e/libcxx/include/__locale_dir/locale_base_api.h#L116-L138 This adds Emscripten in the beginning of the "old" API list. (This has to be the beginning; see #23414) - Import libc headers used by libcxx: 12a4ee4, 12a4ee4 and 43c8ce4 This imports a part of libc headers into `system/lib/llvm-libc`. The imported directories are: - `libc/shared` - `libc/src/__support` - `libc/include/llvm-libc-macros` - `libc/include/llvm-libc-types` - `libc/hdr` See "sharing of headers between libc+ and libc" above for details. This also applies llvm/llvm-project#133999, which is a bugfix that has not be backported, which fixes the bug of including from a wrong directory. - `std::uncaught_exception` -> `std::uncaught_exceptions`: 1bf4e78 `std::uncaught_exception` has been deprecated in C++17, so it generates a warning (which we treat as an error). Replaced it with `std::uncaught_exceptions`, which returns the number of uncaught exceptions (but can still be used as a boolean in the test). - Remove `ryu_constants.h` and `ryu_long_double_constants.h` from libc: 5767ac4 These are not used from libcxx and `ryu_long_double_constants.h` is huge (12M).
libc++ was assuming that musl doesn't have
xlocale.h
which is true, but emscripten adds xlocale.h for compatibility (I'm not sure we really need to, but we do).Fixes: #23413